Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refresh plugin for August 2023 #405

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Refresh plugin for August 2023 #405

wants to merge 13 commits into from

Conversation

basil
Copy link
Member

@basil basil commented Aug 23, 2023

Update this plugin to conform to the latest best practices for Jenkins plugins from the Jenkins archetype, including building/testing on Java 17/21. This entailed migrating from PowerMock to Mockito 5.

@basil basil added the internal label Aug 23, 2023
@basil basil requested a review from a team August 23, 2023 22:11
@basil basil marked this pull request as ready for review August 23, 2023 22:11
@@ -2,6 +2,6 @@
<extension>
<groupId>io.jenkins.tools.incrementals</groupId>
<artifactId>git-changelist-maven-extension</artifactId>
<version>1.2</version>
<version>1.7</version>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest version at the time of this writing.

@@ -5,7 +5,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>4.31</version>
<version>4.72</version>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest version at the time of this writing. Supports Java 11 source level.

@@ -17,13 +17,10 @@
<properties>
<revision>2.7.2</revision>
<changelist>-SNAPSHOT</changelist>
<jenkins.version>2.277.2</jenkins.version>
<java.level>8</java.level>
<jenkins.version>2.387.3</jenkins.version>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

<aws-credentials.version>1.32</aws-credentials.version>
<mockito.version>3.12.4</mockito.version>
<powermock.version>2.0.5</powermock.version>
<aws-sdk.version>1.12.529-406.vdeff15e5817d</aws-sdk.version>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest version at the time of this writing.

<mockito.version>3.12.4</mockito.version>
<powermock.version>2.0.5</powermock.version>
<aws-sdk.version>1.12.529-406.vdeff15e5817d</aws-sdk.version>
<aws-credentials.version>191.vcb_f183ce58b_9</aws-credentials.version>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latest version at the time of this writing.

Comment on lines +148 to +169
<profiles>
<profile>
<id>jdk17</id>
<activation>
<jdk>[17,)</jdk>
</activation>
<properties />
<build>
<pluginManagement>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<configuration>
<argLine>-Xms768M -Xmx768M -XX:+HeapDumpOnOutOfMemoryError -XX:+TieredCompilation -XX:TieredStopAtLevel=1 @{jenkins.addOpens} @{jenkins.insaneHook} --add-opens java.base/java.util.concurrent=ALL-UNNAMED --add-opens java.base/java.util.concurrent.locks=ALL-UNNAMED</argLine>
</configuration>
</plugin>
</plugins>
</pluginManagement>
</build>
</profile>
</profiles>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code also appears in the jira-plugin build, to allow the use of Mockito in integration tests.

Comment on lines 70 to 71
@VisibleForTesting
static List<Cloud> getClouds() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed for the switch from PowerMock to Mockito's mockStatic.

Comment on lines +57 to +58
@VisibleForTesting
static List<Widget> getWidgets() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed for the switch from PowerMock to Mockito's mockStatic.

Comment on lines +68 to +69
@VisibleForTesting
static List<Cloud> getClouds() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed for the switch from PowerMock to Mockito's mockStatic.

EU_WEST_1("eu-west-1", "EU (Ireland)"),
EU_WEST_2("eu-west-2", "EU (London)"),
EU_WEST_3("eu-west-3", "EU (Paris)"),
EU_CENTRAL_1("eu-central-1", "EU (Frankfurt)"),
EU_CENTRAL_2("eu-central-2", "EU (Zurich)"),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed to upgrade to the latest AWS Java SDK Jenkins plugin.


final AmazonAutoScalingClient result = new AutoScalingGroupFleet().createClient(null, REGION, ENDPOINT);
assertEquals(autoScalingClient, result);
try (MockedConstruction<AmazonAutoScalingClient> mockedAmazonAutoScalingClient = Mockito.mockConstruction(AmazonAutoScalingClient.class)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and elsewhere, review with "Hide whitespace" enabled.

@basil
Copy link
Member Author

basil commented Nov 8, 2023

@pdk27

@basil
Copy link
Member Author

basil commented Nov 17, 2023

@pdk27 pdk27 removed their request for review January 12, 2024 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant